Hi @2010YOUY01, I've submitted the pilot pr for bitwise_xor as we discussed. Looking forward to your feedback!

The implementation logic for bitwise_or and bitwise_and is very similar to bitwise_xor. If this approach is acceptable, I plan to refactor those operators using macros in a follow-up step.

At what phase of planning do we do this replacement? I have a concern that we have optimizer rules based around binary exprs that are BitwiseXor ops that might be made invalid if we substitute it with a scalar UDF 🤔

Hi @Jefffrey, thanks for the feedback!

My apologies, I didn't consider this thoroughly enough. I realize now that making this change directly (during the initial conversion from statement to LogicalPlan) would break the optimizer. For instance, I haven't even migrated the logic from simplify_expr to the UDF's simplify method yet.

I will convert this pr to a draft for now and carefully study the impact of this change on existing optimization rules.

Thank you for this POC; it helps clarify the issue.

I see two benefits to unifying operators and UDFs:

1. Optimizer rules would no longer need to differentiate between expressions and UDFs, which could simplify many optimizer implementations.
2. A unified Operator/UDF API: as described in Unifying Operator Handling with the Scalar Function Framework #20018, we already have good signature checking and user-friendly error message utilities for UDFs, but not for operators.

This PR’s approach is to replace operator inside Expr with UDFs during the SQL -> Logical Plan stage. Conceptually, I think this is the ideal way operator expr should have been implemented from the start. However, doing this now would require migrating all related optimizer implementations for that operator. Based on some code searching, I believe this is effectively infeasible: changes that involve deep optimizer refactoring are particularly hard to review, and at the moment there likely isn’t enough review capacity for such a large and complex change.

As a middle ground, I have another idea (this gives up 1 for the reason above, but still tries to achieve 2): operator structs that implement PhysicalExpr could be thin wrappers around ScalarUDF, for example:

#[derive(Debug, Clone, Eq)]
pub struct BinaryExpr {
    left: Arc<dyn PhysicalExpr>,
    op: Operator,
    right: Arc<dyn PhysicalExpr>,
    /// Specifies whether an error is returned on overflow or not
    fail_on_overflow: bool,
    inner: Arc<dyn ScalarUDF>,
}

Here, inner would handle the heavy lifting such as signature checking and execution, while the outer BinaryExpr would mainly delegate to inner to implement PhysicalExpr.

This way, operators and UDFs could share a consistent interface and reuse utilities like signature checking and user-friendly error messages. I think this approach is feasible, though I may be underestimating the difficulty since I haven’t worked extensively in these modules; I believe @Jefffrey has a deeper understanding here.